Skip to content

ci(v1.x): sync the #91 cancellation migration - #220

Merged
heskew merged 4 commits into
v1.xfrom
sync-cancellation-v1.x
Sep 1, 2026
Merged

ci(v1.x): sync the #91 cancellation migration#220
heskew merged 4 commits into
v1.xfrom
sync-cancellation-v1.x

Conversation

@heskew

@heskew heskew commented Aug 28, 2026

Copy link
Copy Markdown
Member

Brings the reusable-owned-cancellation migration (#219) to v1.x — byte-identical to main once #219 merges, so merge #219 first, then this (caller validation compares against the default branch).

🤖 Generated with Claude Code

https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt

Content matches fleet-cancellation-migration (#219) — byte-identical
to main once that merges.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
@heskew
heskew requested a review from a team as a code owner August 28, 2026 23:05
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

2 blockers found. Prior 2 finding(s) remain.

1. Missing events in opt-in gate (Claude)

File: .github/workflows/claude-review.yml:36
What: The if: gate for the review job still misses synchronize, opened, and reopened events when the claude-review label is present but vars.CLAUDE_ALWAYS_ON is false.
Why it matters: PRs that opt into AI reviews via the label will not receive automated reviews upon creation or re-reviews on subsequent pushes. The recent addition of ready_for_review to the gate is incomplete and doesn't cover the other common pull request actions for opt-in users.
Suggested fix: Apply the simplified, robust logic proposed in the prior review:

if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'claude-review') || (github.event.action != 'labeled' && (vars.CLAUDE_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'claude-review'))) }}

2. Missing events in opt-in gate (Gemini)

File: .github/workflows/gemini-review.yml:44
What: The if: gate for the review job still misses synchronize, opened, and reopened events when the gemini-review label is present but vars.GEMINI_ALWAYS_ON is false.
Why it matters: Similar to the Claude workflow, opt-in users will miss automated reviews and re-reviews on subsequent pushes. The ready_for_review addition does not solve the underlying issue flagged in the prior round.
Suggested fix: Apply the simplified, robust logic proposed in the prior review:

if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'gemini-review') || (github.event.action != 'labeled' && (vars.GEMINI_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'gemini-review'))) }}

heskew and others added 2 commits August 28, 2026 16:21
Review feedback on oauth#219 — consistency with the PR's permissions-
ceiling goal, applied fleet-wide.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
The retained note claiming the label is matched only in the reusable
contradicted the caller gate two lines below (it now names the label);
reworded to state the deliberate duplication. Narrating comments that
restated the adjacent declaration are dropped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
@heskew

heskew commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Codex review: no blockers; byte-identical port of #219. One finding: a review-lane concurrency race (an older push's stale review can cancel a newer one), tracked upstream as ai-review-prompts#92 and likely behind the flaky duplicate review runs. Not a release issue; fine to land with #92 as follow-up.

Kris's precondition for this migration (harper#2385 thread,
ai-review-prompts#92/#94): stale runs are refused admission to the
review job's cancelling group and self-fail if superseded after
acquiring the slot, so removing the caller-side cancellation cannot
let an older run cancel a newer admitted review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
Comment thread .github/workflows/claude-review.yml
Comment thread .github/workflows/gemini-review.yml

@heskew heskew left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-check at c646b7e: no blockers.

All five changed caller workflows are byte-for-byte identical to current main after #219. The ai-review-prompts #94 pin is exact and consistent across uses/inputs; permission ceilings, label/ALWAYS_ON policy, pre-queue freshness gating, reusable-owned cancellation, and the post-acquire stale-head check all line up. I honored the two resolved label-gate threads as the accepted cost policy.

The red Claude review job is historical: it ran before #219 merged and failed because this caller did not yet match the default branch. That prerequisite is now satisfied, so a rerun should clear it. Node 22/24, Bun, validation, and security checks are green.

@heskew
heskew merged commit 3f9e1d8 into v1.x Sep 1, 2026
13 of 14 checks passed
@heskew
heskew deleted the sync-cancellation-v1.x branch September 1, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant